-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert trusted actions list to data extension #18435
base: main
Are you sure you want to change the base?
Convert trusted actions list to data extension #18435
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.
Files not reviewed (2)
- actions/ql/lib/codeql/actions/security/UseOfUnversionedImmutableAction.qll: Language not supported
- actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql: Language not supported
Comments suppressed due to low confidence (1)
actions/ql/lib/codeql/actions/security/owner/trusted/trusted_actions_owner.model.yml:2
- There is an extra space after the colon. It should be consistent with the rest of the file.
- addsTo:
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed repo
to nwo
to be precise in the naming used here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change. Could you also update the associated qhelp to describe to users how that can add their own data extension to customize this query?
- use existing data extensions config and yml folder - rename from trustedActionsOwner to trustedActionsOwnerDataModel - update related predicates
I actually had this same thought when creating that extension. Would make the feature easily discoverable! I took a stab at it: codeql/actions/ql/src/Security/CWE-829/UnpinnedActionsTag.md Lines 12 to 45 in 6b3098d
|
- models/**/*.yml | ||
``` | ||
|
||
3. Ensure that the model pack is included in your CodeQL analysis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step is not possible for default setup. I believe it's possible to just place the extensions file in .github/codeql/extensions
in the current repository and have it automatically included in the run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to setup model packs with org level setup: Extending CodeQL coverage with CodeQL model packs in default setup ... added to references link at the bottom of the .md
I struggle with how verbose to make this guidance in the query markdown, feels like it would be better documented out in the (yet to be released) https://codeql.github.com/docs/codeql-language-guides/codeql-for-actions/
I will take a stab at adding some of the additional options here as a hint in the right direction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm...that might be a better use of space to link out to the language guide (when it is available). I am a little concerned that if we load up the qhelp with lots of information around configuration, then autofix will get confused.
Presumably, the language docs are released along with the CLI release. So, timing would work out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point about autofix, I will shift gears and move these docs in that direction and consider deep linking via reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the changelog a bit more verbose as a compromise.
Having an inventory of which queries are extensible via data extensions (and which scenarios) + including that in the query docs would be a larger effort that other queries would benefit from.
For future docs considerations:
Configuration
If there is an Action publisher that you trust, you can include the owner name/organization in a data extension model pack to add it to the allow list for this query. Adding owners to this list will prevent security alerts when using unpinned tags for Actions published by that owner.
Example
To allow any Action from the publisher
octodemo
, such asoctodemo/3rd-party-action
, follow these steps:
Create a data extension file
/models/trusted-owner.model.yml
with the following content:extensions: - addsTo: pack: codeql/actions-all extensible: trustedActionsOwnerDataModel data: - ["octodemo"]Create a model pack file
/codeql-pack.yml
with the following content:name: my-org/actions-extensions-model-pack version: 0.0.0 library: true extensionTargets: codeql/actions-all: '*' dataExtensions: - models/**/*.ymlEnsure that the model pack is included in your CodeQL analysis.
By following these steps, you will add
octodemo
to the list of trusted Action publishers, and the query will no longer generate security alerts for unpinned tags from this publisher.References
The actions/unpinned-tag query can be noisy under certain circumstances (ex: many internally developed actions) where unpinned actions is an acceptable risk. Today, there is no capability to modify the trusted owners (currently hardcoded to
github, actions, advanced-security
).This change will make the query extensible so that a extension pack (which supports default setup at scale) could define additional trusted Actions owners. Ex:
Which would expand the default set:
codeql/actions/ql/lib/ext/config/trusted_actions_owner.yml
Lines 1 to 8 in bccec0a
Pull Request checklist
All query authors
.qhelp
. See the documentation in this repository.Internal query authors only
.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).